Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add LatestHead relayer method to gRPC client and server implementations #767

Merged
merged 4 commits into from
Sep 16, 2024
Merged

Add LatestHead relayer method to gRPC client and server implementations #767

merged 4 commits into from
Sep 16, 2024

Conversation

kidambisrinivas
Copy link
Contributor

Updates

  • Add LatestHead relayer function to gRPC client-server interfaces and implementations
    • Add LatestHead implementation to relayerset server and clients

Build

$ go generate ./...

Testing

$ go test -count=1 -v ./pkg/loop/internal/relayerset
=== RUN   Test_RelayerSet
2024-09-13T11:08:35.420+0100 [DEBUG] accepting initial connection: addr=/var/folders/1b/byq7w64n1x7gmp5cp9__vp_r0000gn/T/plugin3130643913
2024-09-13T11:08:35.420+0100 [DEBUG] stdio EOF, exiting copy loop
2024-09-13T11:08:35.420+0100 [DEBUG] stdio EOF, exiting copy loop
2024-09-13T11:08:35.421+0100 [DEBUG] making new client mux initial connection: addr=/var/folders/1b/byq7w64n1x7gmp5cp9__vp_r0000gn/T/plugin3130643913
2024-09-13T11:08:35.421+0100 [DEBUG] initial server connection accepted: addr=/var/folders/1b/byq7w64n1x7gmp5cp9__vp_r0000gn/T/plugin3130643913
2024-09-13T11:08:35.421+0100 [DEBUG] client muxer connected: addr=/var/folders/1b/byq7w64n1x7gmp5cp9__vp_r0000gn/T/plugin3130643913
2024-09-13T11:08:35.421+0100 [DEBUG] sending conn to default listener
2024-09-13T11:08:35.424+0100 [DEBUG] sending conn to default listener
2024-09-13T11:08:35.424+0100 [DEBUG] stdio: received EOF, stopping recv loop: err="rpc error: code = Unavailable desc = error reading from server: EOF"
--- PASS: Test_RelayerSet (0.00s)
PASS
ok      github.com/smartcontractkit/chainlink-common/pkg/loop/internal/relayerset       0.398s

@kidambisrinivas kidambisrinivas requested a review from a team as a code owner September 13, 2024 10:08
@kidambisrinivas kidambisrinivas requested review from ilija42 and removed request for a team September 13, 2024 10:08
@kidambisrinivas kidambisrinivas changed the title Add LatestHead relayer function to gRPC client-server interfaces and implementations Add LatestHead relayer method to gRPC client and server implementations Sep 13, 2024
@ilija42 ilija42 requested review from a team and jmank88 and removed request for a team September 13, 2024 13:00
jmank88
jmank88 reviewed Sep 13, 2024
View reviewed changes
pkg/loop/internal/pb/relayerset/relayerset.proto Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we have been mixing field name styles:

  • legacy_url
  • transmitterID
  • relayerId
  • plugin_provider_id

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kidambisrinivas Do you mind following up on Jordan's comment, either in this PR or in the next?

Copy link
Contributor Author

@kidambisrinivas kidambisrinivas Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In gRPC code apparently, this style is the convention: relayerId, which is what I've been using in general.
Should I fix the rest of the name styles?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can fix them without affecting the generated Go code, then definitely yes. If it changes the generated Go code, then probably still yes, but we can reconsider if the effect is too disruptive.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it'll affected generated go code and would likely be disruptive. I could handle this as a separate PR, as the naming styles are prior to this PR and would be a larger change in general

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In gRPC code apparently, this style is the convention: relayerId

Do you have a reference for this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eh I guess I see Id here: https://protobuf.dev/getting-started/gotutorial/#protobuf-api

jmank88
jmank88 previously approved these changes Sep 13, 2024
View reviewed changes
@kidambisrinivas kidambisrinivas merged commit b8f8ccc into main Sep 16, 2024
9 of 11 checks passed
@kidambisrinivas kidambisrinivas deleted the relayer-interface-latesthead branch September 16, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmank88 jmank88 jmank88 approved these changes

@ilija42 ilija42 ilija42 approved these changes

@cedric-cordenier cedric-cordenier cedric-cordenier approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kidambisrinivas @jmank88 @cedric-cordenier @ilija42